Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modification by filter (without formula data migration) #526

Merged
merged 56 commits into from
Oct 3, 2024

Conversation

thangqp
Copy link
Contributor

@thangqp thangqp commented Sep 2, 2024

No description provided.

@thangqp
Copy link
Contributor Author

thangqp commented Oct 1, 2024

I have made a voluntary invalid assignment in my second filter edit line:

hiérarchie reports hiérarchie reports

As you can see

* The error is not in the right report (it is in "Modification by filter" instead of "Assignment on filters : batteries à moins de 25 kV", which is empty)

* The message "Assignment on filters : batteries à moins de 25 kV" has a strange "unknown" severity. Shouldn't it be an INFO ?

* there is only one error (even if I was assigning this value to several batteries)

At the moment I will keep the actual behaviour as formula, i.e. when exception occurs, do not interrupt next applications and log as warning for exception

https://github.com/gridsuite/network-modification-server/pull/526/files#diff-793232bebcbb224f95ef5d7c306f29898390e54e86f4e68cc792e6724f393a96R310-R318

@Mathieu-Deharbe
Copy link
Contributor

At the moment I will keep the actual behaviour as formula, i.e. when exception occurs, do not interrupt next applications and log as warning for exception

https://github.com/gridsuite/network-modification-server/pull/526/files#diff-793232bebcbb224f95ef5d7c306f29898390e54e86f4e68cc792e6724f393a96R310-R318

Ok fine. But I think a new ticket should be opened about this. It makes the logs very misleading :

image

It really looks like no equipment has been modified at all but it is only in one of the Assigment by filter. The other worked fine.

@thangqp
Copy link
Contributor Author

thangqp commented Oct 2, 2024

At the moment I will keep the actual behaviour as formula, i.e. when exception occurs, do not interrupt next applications and log as warning for exception
https://github.com/gridsuite/network-modification-server/pull/526/files#diff-793232bebcbb224f95ef5d7c306f29898390e54e86f4e68cc792e6724f393a96R310-R318

Ok fine. But I think a new ticket should be opened about this. It makes the logs very misleading :

image

It really looks like no equipment has been modified at all but it is only in one of the Assigment by filter. The other worked fine.

In yours case, none of assignments done well, that why the final counter is zero so the parent node is in error severity.

I tested with two modifications, one no error and other with error, I dont see any problem with the counter:
SomeError

@Mathieu-Deharbe
Copy link
Contributor

Mathieu-Deharbe commented Oct 3, 2024

In yours case, none of assignments done well, that why the final counter is zero so the parent node is in error severity.

This is what bothers me : the parent is in error severity but the node when the error actually happened has no error. And there is no intuitive way to find where the error happenned : you have to click on the nodes one by one and checks the traces.

@thangqp
Copy link
Contributor Author

thangqp commented Oct 3, 2024

In yours case, none of assignments done well, that why the final counter is zero so the parent node is in error severity.

This is what bothers me : the parent is in error severity but the node when the error actually happened has no error. And there is no intuitive way to find where the error happenned : you have to click on the nodes one by one and checks the traces.

We can set error severity when exception occurs here : https://github.com/gridsuite/network-modification-server/pull/526/files#diff-793232bebcbb224f95ef5d7c306f29898390e54e86f4e68cc792e6724f393a96R317
But I don't know whether it is good solution. In my opinion, error means no next application, we can merge and see Christelle in the next week

Copy link
Contributor

@ghazwarhili ghazwarhili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code review => OK

Copy link

sonarqubecloud bot commented Oct 3, 2024

@thangqp thangqp merged commit ffd24f7 into main Oct 3, 2024
3 checks passed
@thangqp thangqp deleted the modification_by_filter_no_migration_data branch October 3, 2024 14:19
EtienneLt pushed a commit that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants